Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify --prefix for npm install command for typescript and typescript-language-server #62

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

jetzhou
Copy link
Contributor

@jetzhou jetzhou commented Feb 12, 2025

In certain setups, multilspy might be installed at a location where a parent directory has a node_modules folder already. For example, in a docker container, multilspy will most likely just be under /usr/local/lib/python and if there is any npm installed global dependencies, there will also be a /usr/local/lib/node_modules folder.

In such a setup, npm install typescript will put the executable under /usr/local/lib/node_modules when setup_runtime_dependencies is run for typescript repos. This will cause the assert for checking typescript-language-server being located under multilspy/language_servers/typescript_language_server/static/ts-lsp to fail right after the installation.

We can fix this issue by adding --prefix ./ to the npm install command in the settings file. This PR implements that fix.

@jetzhou
Copy link
Contributor Author

jetzhou commented Feb 12, 2025

@microsoft-github-policy-service agree company="Parasail Software, Inc"

@themichaelusa
Copy link
Contributor

LGTM, ty

@camsteffen
Copy link

Was going to open a PR with the exact same change and then I found this! 😄

@LakshyAAAgrawal LakshyAAAgrawal merged commit 9dfe2c0 into microsoft:main Feb 18, 2025
6 checks passed
@LakshyAAAgrawal
Copy link
Collaborator

Dear @jetzhou , Thank you so much for catching this and providing a fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants